Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adjusted "selector" option #14538

Closed
wants to merge 1 commit into from
Closed

Adjusted "selector" option #14538

wants to merge 1 commit into from

Conversation

iSimonWeb
Copy link

Made this edit to make this.selector match every anchor whose href attribute starts with an # symbol.
It is useful for me (and for everyone else) to match the anchor of the logo in my navbar with the first section (#home) of the onepage I'm currently developing.

@iSimonWeb
Adjusted "selector" option
349379a 
Made this edit to make this.selector match every anchor whose href attribute starts with an # symbol.
It is useful for me (and for everyone else) to match the anchor of the logo in my navbar with the first section (#home) of the onepage I'm currently developing.
@hnrch02
Copy link
Collaborator

hnrch02 commented Sep 5, 2014

Don't think we can change this in v3, it'd be a breaking change.

@iSimonWeb
Copy link
Author

@hnrch02 are you sure? The adding of .nav li > a at the end of the option.target was neither mentioned in the docs!
Secondly can I ask you why the test failed? The error that has been thrown isn't in my code but in the setup process of the test. I'm a bit confused.

@hnrch02
Copy link
Collaborator

hnrch02 commented Sep 5, 2014

Yeah, it's not explicitly stated, only implied:

To easily add scrollspy behavior to your topbar navigation, add data-spy="scroll" to the element you want to spy on (most typically this would be the <body>). Then add the data-target attribute with the ID or class of the parent element of any Bootstrap .nav component. [Emphasis mine]

However it could be behavior people are relying upon and changing that would be a breaking change, IMO.

Regarding the tests, they fail from time to time, maybe someone who has access will re-run them at some point.

@cvrebert cvrebert added css and removed css labels Sep 5, 2014
@iSimonWeb
Copy link
Author

Got your point now, we'll see what they think about that :)

@hnrch02
Copy link
Collaborator

hnrch02 commented Sep 10, 2014

Closing per #13050 (comment). Also, this is planned for v4.

@hnrch02 hnrch02 closed this Sep 10, 2014
@iSimonWeb iSimonWeb deleted the patch-1 branch August 27, 2017 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
js
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@iSimonWeb @hnrch02 @cvrebert